Skip to content

Conversation

@gregoire-dl
Copy link
Member

@gregoire-dl gregoire-dl commented Sep 8, 2025

Description

Previously, all attributes were implicitly treated as non-keyable.
This PR introduces support for keyable attributes, allowing them to be loaded, stored, animated or manipulated.
Backward compatibility is preserved.

testKeyable
A test node with keyable attributes.

Current limitations and scope:

  • Keyable support is implemented for: BoolParam, IntParam, and FloatParam.
  • Only viewId is currently supported as the keyType.
  • Interpolation mechanisms are not yet implemented.

Unit tests cover:

  • Creation and initialization of keyable attributes.
  • Modification and deletion of keyable attributes.
  • Linked keyable attribute.
  • Keyable attribute UID.

Sample Node:

A sample node for testing: TestNode.zip

Features list

  • New KeyValues class to manage the keyable attribute list of (key, value) pairs.
  • Add keyable support in attribute description.
  • Add keyable support for: BoolParam, IntParam, and FloatParam.
  • Graph commands for keyable attribute.
  • UI integration in AttributeItemDelegate.qml
  • Unit tests

Implementation remarks

This PR is designed in the context of developing the new shape editor.
It can serve as a foundation for full support of keyable attributes in the future.

@gregoire-dl gregoire-dl added this to the Meshroom 2026.1.0 milestone Sep 8, 2025
@gregoire-dl gregoire-dl self-assigned this Sep 8, 2025
@gregoire-dl gregoire-dl added feature new feature (proposed as PR or issue planned by dev) type:pr pull request issue labels Sep 8, 2025
@codecov
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

❌ Patch coverage is 95.38462% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.06%. Comparing base (000d91e) to head (9149d0a).
⚠️ Report is 63 commits behind head on develop.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
meshroom/core/attribute.py 88.88% 5 Missing ⚠️
meshroom/core/keyValues.py 91.37% 5 Missing ⚠️
meshroom/core/desc/attribute.py 85.71% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2878      +/-   ##
===========================================
+ Coverage    79.52%   80.06%   +0.53%     
===========================================
  Files           51       53       +2     
  Lines         6976     7225     +249     
===========================================
+ Hits          5548     5785     +237     
- Misses        1428     1440      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gregoire-dl gregoire-dl added the wip work in progress label Sep 26, 2025
Allows to store a list of pairs (key, value) based on an attribute description.
- display value or default value at current viewId for keyable attribute.
- allows to set/update current viewId value for keyable attribute.
- new add/remove key button for keyable attribute.
- check keyable attribute initialization.
- check keyable attribute CRUD.
- check keyable attribute linked.
- check keyable attribute UID.
Fix logic error, Attribute.getDefaultValue() is used in node serialization and represents the true default value of an attribute. In the case of a keyable attribute this should be an empty dict (no keys).
We should use attribute description to get the default value of a key.
In this case, we can use SetAttributeCommand with attribute.getDefaultValue().
@gregoire-dl gregoire-dl added review and removed wip work in progress labels Sep 26, 2025
checked: attribute.keyable && attribute.keyValues.hasKey(_reconstruction.selectedViewId)
enabled: root.editable
onClicked: {
if (attribute.keyValues.hasKey(_reconstruction.selectedViewId))
Copy link
Member

@fabiencastan fabiencastan Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should do nothing if there is no selected view ("selectedViewId" is "-1").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe globally lock the attribute when there is no selectedViewId.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
I made the changes to lock the attribute.
In any case, it's not possible to add an attribute value for a negative key, checked in KeyValues.add().

@cbentejac cbentejac merged commit 3e1d496 into develop Oct 3, 2025
3 of 4 checks passed
@cbentejac cbentejac deleted the dev/attributeValues branch October 3, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature new feature (proposed as PR or issue planned by dev) review type:pr pull request issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants